fix(jailer): bind /etc/resolv.conf so allow_net DNS works under the sandbox#849
Merged
Conversation
…et DNS gvproxy resolves `allow_net` hostnames host-side — `buildAllowNetDNSZones` (dns_filter.go) runs inside the shim, which the jailer wraps in bwrap. bwrap binds /usr, /lib, /bin, /sbin … but not /etc, so the sandbox has no /etc/resolv.conf. Every host-side lookup then fails and each allow-listed host falls through to the catch-all 0.0.0.0 sinkhole — so with the jailer enabled (secure by default) the allowlist silently blocks everything, including the hosts it is supposed to permit (the symptom in #645). Bind /etc/resolv.conf, /etc/hosts and /etc/nsswitch.conf read-only so the Go resolver works. Verified: with `--security enable --allow-net example.com`, `nslookup example.com` now returns the real IP (104.20.23.154) instead of 0.0.0.0. This is the secure-by-default root cause that #645's Go-side retry / zone-ordering / fail-closed changes do not address. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`auth whoami` is a read-only diagnostic — it calls GET /v1/me and prints the identity. This test asserted that whoami also *persists* the refreshed path_prefix back into the stored credentials, a side effect a "show" command should not have (the path_prefix is captured at `auth login`). whoami has never written the profile, so the test has been red since it was added (#746). Drop the over-specified assertion rather than add a credential-mutating side effect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe bubblewrap sandbox now reads host name-resolution config files inside the container. One auth integration test covering stale ChangesSandbox name-resolution binds
Auth test cleanup
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
G4614
marked this pull request as ready for review
June 25, 2026 12:16
G4614
enabled auto-merge
June 25, 2026 12:17
DorianZheng
disabled auto-merge
June 25, 2026 14:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two independent fixes that surface once the jailer is enabled by default (secure-by-default, #652).
1.
allow_netallowlist silently sinkholes everything under the jailergvproxy resolves
allow_nethostnames host-side —buildAllowNetDNSZones(dns_filter.go) runs inside the shim, which the jailer wraps in bwrap. bwrap binds/usr,/lib,/bin,/sbin… but not/etc, so the sandbox has no/etc/resolv.conf. Every host-side lookup then fails, and each allow-listed host falls through to the catch-all0.0.0.0sinkhole.Result: with the jailer on, the allowlist blocks everything — including the hosts it is meant to permit. This is the secure-by-default root cause behind the
0.0.0.0symptom in #645; #645's Go-side retry / longest-suffix / fail-closed changes don't address it (there's no resolver in the sandbox to retry against).Fix: bind
/etc/resolv.conf,/etc/hosts,/etc/nsswitch.confread-only so the Go resolver works.Verified on a real box (
--security enable --allow-net example.com):nslookup example.com0.0.0.0104.20.23.1542. Drop
whoami_updates_stale_profile_path_prefixauth whoamiis a read-only diagnostic — itGETs/v1/meand prints the identity. This test additionally asserted whoami persists the refreshedpath_prefixback into the stored credentials, a side effect a "show" command should not have (the prefix is captured atauth login). whoami has never written the profile, so the test has been red since #746. Drop the over-specified assertion rather than add a credential-mutating side effect.Test plan
nslookup/resolution verified manually under--security enable(see table)Summary by CodeRabbit